Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not reset the ORDERLABEL if division is a page #6238

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BartChris
Copy link
Collaborator

@BartChris BartChris commented Sep 27, 2024

fixes #5220
alternative: #6244
We should not reset the ORDERLABEL of a page during metadata preservation since this breaks the pagination when the ORDERLABEL is set to excluded.
While this fixes the pagination issue there are other potential problems. I did an alternative pull request which tries to adress them all:
#6244

@BartChris BartChris marked this pull request as draft September 29, 2024 15:47
@BartChris
Copy link
Collaborator Author

BartChris commented Sep 30, 2024

Although i did not yet notice any problems when not resetting the ORDERLABEL for pages (as done in this PR), we can also extend the original logic. The root cause of the linked issue is, that the necessary logic for setting the LABEL and ORDERLABEL (which are set to NULL in the beginning) is not called for hidden metatdata.

Pair<BiConsumer<Division<?>, String>, String> metsFieldValue = row.getStructureFieldValue();
if (Objects.nonNull(metsFieldValue)) {
metsFieldValue.getKey().accept(division, metsFieldValue.getValue());

To solve the issue at hand we could for hiddenMetadata call the setters directly to not lose the ORDERLABELs and therefor our pagination. We would by this also preserve hidden division LABEL's, which might be lost.

private void processHiddenMetadata() {
        for (Metadata metadatum : hiddenMetadata) {
            if (metadatum instanceof MetadataEntry) {
                MetadataEntry metadataEntry = (MetadataEntry) metadatum;
                String key = metadataEntry.getKey().toUpperCase();
                String value = metadataEntry.getValue();
                switch (key) {
                    case "LABEL":
                        division.setLabel(value);
                        break;
                    case "ORDERLABEL":
                        division.setOrderlabel(value);
                        break;
                    case "CONTENTIDS":
                        division.getContentIds().add(URI.create(value));
                        break;
                }
            }
        }
        metadata.addAll(hiddenMetadata);
    }

A more general remark: The necessary setters for the divisions of (non-hidden) metadata elements in the UI are called as a side effect of the (IMHO very complicated) BiConsumer logic, which does some transformations to the values from the tree, but mainly defines the setters for different division values, which are then applied. There may be a specific reason for using functional interfaces here, but I wonder if there might be opportunities to simplify the logic for better readability and maintainability.

@solth
Copy link
Member

solth commented Oct 1, 2024

@BartChris Please rebase against current master to avoid failing builds caused by broken Selenium tests!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting the ORDERLABEL to excluded breaks the pagination
2 participants